-
Couldn't load subscription status.
- Fork 239
feat(compass-aggregations): disable search stages for incompatible views COMPASS-9693 #7217
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
packages/compass-aggregations/src/components/stage-toolbar/stage-operator-select.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for search stages in aggregation views with version and compatibility checks. The changes implement conditional disabling of search stages based on MongoDB version compatibility and view pipeline queryability requirements.
- Adds version compatibility checks for search stages on views (MongoDB 8.0+/8.1+ requirement)
- Implements pipeline queryability validation for search index compatibility
- Updates collection stats to include pipeline information for view compatibility checks
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/compass-aggregations/src/modules/collection-stats.ts | Adds pipeline field to collection stats to support view compatibility checks |
| packages/compass-aggregations/src/components/stage-toolbar/stage-operator-select.tsx | Implements search stage validation logic with version checks and dynamic descriptions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Can you explain somewhere why DE and Compass have different minimum server versions? |
Sorry I misunderstood something but yes you're right DE itself isn't related to server version 8.1+. ̶W̶e̶ ̶a̶r̶e̶ ̶n̶o̶t̶ ̶d̶i̶s̶p̶l̶a̶y̶i̶n̶g̶ ̶t̶h̶e̶ ̶s̶e̶a̶r̶c̶h̶ ̶s̶t̶a̶g̶e̶s̶ ̶i̶n̶ ̶D̶a̶t̶a̶ ̶E̶x̶p̶l̶o̶r̶e̶r̶,̶ ̶o̶n̶l̶y̶ ̶C̶o̶m̶p̶a̶s̶s̶. I removed the extra check and made it only check Compass version for compatibility. |
|
Hey @DarshanaVenkatesh, can you clarify, if we don't need to differenciate by version here, does it also mean we didn't need to do it in this PR? Because Le Roux is basically asking the same thing I asked you before, so just want to make sure we're not introducing unexpected differences in behavior for desktop and web that otherwise in most cases always behave the same |
That pr should differentiate in the banners because we want to direct users to the atlas search indexes page for data explorer. So yes, we want banners with the server version difference because that's mentioning the server version in Atlas vs Compass (not necessarily data explorer). |
| // filter out search stages for data explorer | ||
| const filteredStages = | ||
| isReadonlyView && !enableAtlasSearchIndexes | ||
| ? stages.filter((stage) => !isSearchStage(stage.name)) | ||
| : stages; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is this requirement coming from? This is a weird change in compass behavior only for data explorer. Why are you not allowed to see actually working stages in aggregation builder based on a completely different feature?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing this! Sorry for the back and forth. The final implementation will have both of them work the same for compass and de! Will ping you after I update this pr but both will have search stages with the same messages.
Description
Disable search stages for views depending on version/if incompatible with search.
disable_search_stages.mov
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes